docs: codify when a comment earns its place - #38618
Open
antiguru wants to merge 1 commit into
Open
Conversation
Three rules the peek-execution work produced, added to the code-comment section. A stale comment asserting that `PeekResultIterator` could not be sent to another thread outlived its truth by months. It was read as fact rather than retested, and the peek stash was built to walk the trace a second time because of it. Constraints stated in comments now have to be checked when written. The other two cover what the section left implicit. A sentence earns its place by changing what a maintainer would do, not by being true, and rewording a paragraph while keeping every fact in it is not editing. "X rather than Y" informs only when Y is what a reader would otherwise assume; elsewhere Y is unfalsifiable and rots silently, since the code can move to Y with the sentence still reading correctly. Also narrows "why the obvious alternative was not taken", which reads as licence to justify every choice, to the alternative a reader would assume.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three rules for the code-comment section of
AGENTS.md, and one narrowing of a rule already there.A comment in the compute peek path asserted that
PeekResultIteratorcould not be handed to an async task because the underlying trace reader is notSend. It was wrong. The iterator holdsVec<Tr::Batch>ofArcBatch, never theTraceAgent, and a one-lineassert_sendprobe refutes the claim. The comment was read as fact rather than retested, and the peek stash was built to walk the trace a second time because of it. So a comment asserting a constraint now has to be checked when it is written, by a compile probe, a test, or a pointer to whatever enforces it.The other two cover what the section left implicit. A sentence earns its place by changing what a maintainer would do, not by being true, and rewording a paragraph while keeping every fact in it is not editing.
X rather than Yinforms only where Y is what a reader would otherwise assume; elsewhere Y is unconstrained and unfalsifiable, and it rots silently, since the code can move to Y with the sentence still reading correctly. Where Y is a real trap the failure belongs in the comment instead of the contrast, which gives the claim truth conditions a reader can check.The narrowing is to "why the obvious alternative was not taken", which reads as licence to justify every choice. It now names the alternative a reader would assume, which is the same test the new paragraph applies.
No rule lists banned phrasing. A list of phrases invites working around the list, and the deletion test removes the sentences they live in.
CLAUDE.mdis a symlink toAGENTS.md, so the change lands in both.🤖 Opened by Claude Code on behalf of @antiguru